Fix off-by-one error when centering a transform. #2760
Conversation
JimBobSquarePants
requested review from
tocsoft,
antonfirsov,
dlemstra and
brianpopow
| Matrix3x2 matrix = Matrix3x2.CreateTranslation(-sourceRectangle.Location); | ||
|
|
||
| foreach (Func<Size, Matrix3x2> factory in this.boundsMatrixFactories) |
There was a problem hiding this comment.
| Matrix3x2 matrix = Matrix3x2.CreateTranslation(-sourceRectangle.Location); | |
| foreach (Func<Size, Matrix3x2> factory in this.boundsMatrixFactories) | |
| Matrix3x2 matrix = Matrix3x2.CreateTranslation(-sourceRectangle.Location); | |
| matrix.Translation += new Vector2(1); | |
| foreach (Func<Size, Matrix3x2> factory in this.transformMatrixFactories) | |
| { |
I believe this change should prevent the need of having the 2nd collection of Funcs as I think matrix's for sizing should originate at (+1,+1) from the location of the transform matrix.
There was a problem hiding this comment.
You’ve got me second guessing myself now but I’m not sure that would work. For the most part we want to return the transformed extents based on the explicitly provided information (translate/rotate/scale x, y). It’s only when we perform our own maths to calculate the centre point we go astray. That matrix must be zero based but can then not be used for bounds calculation.
There was a problem hiding this comment.
Confirmed. This example would add an additional unwanted pixel to the right side of the image with the suggested changes.
There was a problem hiding this comment.
thanks for validating, just want to make sure we hadn't missed something more simple, this PR looks fine to me then. 👍
Prerequisites
Description
Fixes #2753
Fixes the off-by-one error introduced when rotating or skewing images.